-
Couldn't load subscription status.
- Fork 13.9k
Skip codegen_crate call in check mode #148035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Some changes occurred in compiler/rustc_codegen_ssa |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Skip codegen_crate call in check mode
This comment has been minimized.
This comment has been minimized.
Sounds allowed by https://rust-lang.github.io/rfcs/3477-cargo-check-lang-policy.html, so no issue so long as it's not skipping anything that would be needed particularly often. (Not reporting things that'll be reported for any full build, like missing flags for a target, seems totally fine to me. That wouldn't be part of the core inner dev loop where check is most important.) |
|
Finished benchmarking commit (3eae4c9): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -1.7%, secondary -3.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -2.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 474.995s -> 475.128s (0.03%) |
3e6df8b to
5a8ffa4
Compare
|
@bors try @rust-timer queue To check how perf.rust-lang.org handles the change in timers. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Skip codegen_crate call in check mode
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (1d990a0): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 474.959s -> 474.684s (-0.06%) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on the idea, and the perf improvements look real.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@bors ping |
|
😪 I'm awake I'm awake |
|
@bors r+ |
|
does this mean that check-mode crater runs will be less effective in the future? |
Slightly, though I'd guess that most of the code that we now avoid running is exercised with a single |
|
I think the only thing that this omits is errors that would be produced by creating the codegen backend. If someone was actually doing crater check runs to find those errors, I have questions. |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 9ea8d67 (parent) -> adaa838 (this PR) Test differencesShow 4 test diffs4 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard adaa838976ff99a4f0661136322f64cb466b58a0 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (adaa838): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary 4.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -3.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 474.628s -> 473.263s (-0.29%) |
|
Seems like it was only the @rustbot label: +perf-regression-triaged |
This way we don't have to spawn the coordinator thread. Some errors will no longer be emitted with this in check mode. For example the check that
-Ctarget-cpuis passed on targets that need this.Suggested by @saethlin